Wallet page #76
Merged
Wallet page #76
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d fiat off-ramping
…to wallet-page Merge into main
📝 WalkthroughWalkthroughAdds a new /wallet page and layout plus multiple wallet UI components (BalanceCard, AssetsList, TransactionHistory, WithdrawalSection, SecuritySection, WalletOverview), wires them to mockWallet data, updates the global navbar, tweaks some UI primitives, and updates mock wallet addresses. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Next as Next.js Router
participant WalletPage as WalletPage
participant Mock as mockWalletWithAssets
participant Components as Wallet Components
Browser->>Next: Request /wallet
Next->>WalletPage: render WalletPage component
WalletPage->>Mock: import mockWalletWithAssets
WalletPage->>Components: pass walletInfo & assets props
Components->>Browser: render BalanceCard, AssetsList, TransactionHistory, WithdrawalSection, WalletOverview
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~23 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@app/wallet/page.tsx`:
- Line 1: The wallet route is missing SEO metadata because app/wallet/page.tsx
is a client component and cannot export metadata; add a server-level metadata
provider by creating app/wallet/layout.tsx that exports a Metadata object (e.g.,
export const metadata: Metadata = { title: "...", description: "..." }) and a
default WalletLayout component that returns children, or alternatively move
static parts of page.tsx into a new server component (e.g., WalletServerPage)
which can export metadata/generateMetadata and then import the client-only
component; update imports so page.tsx is nested under the server wrapper or
include the layout to ensure SEO metadata is applied.
- Around line 55-61: The "Quick Links" block contains anchors with placeholder
href="#" that don't navigate; update the three <a> elements in the "Quick Links"
section (the Quick Links div / its anchor tags) to point to real routes or
external URLs (e.g., /how-it-works, /fee-schedule, /support) or, if the pages
aren’t ready, remove the entire Quick Links block or replace the anchors with
non-interactive placeholders and add a TODO comment indicating the intended
routes; ensure any placeholder anchors are not left with href="#" to avoid
unexpected scrolling or accessibility issues.
- Line 9: Replace the hardcoded mockWalletWithAssets usage by adding a server
API route handler at /app/api/wallet/route.ts that exposes real wallet data
(implement an exported default GET handler named GET or handler that returns
JSON via Next Response) and create a client hook hooks/use-wallet.ts that
follows existing React Query patterns: export a useWallet hook which calls an
async fetchWallet function, registers the query under a stable key like
['wallet', address?], uses react-query's useQuery to fetch from '/api/wallet'
and returns data/error/status; update page.tsx to import and use useWallet
instead of mockWalletWithAssets and remove the mock import. Ensure the
fetchWallet function handles errors and parses JSON, and mirror conventions used
in use-reputation.ts / use-bounties.ts (same queryClient behavior and staleTime
defaults).
In `@components/wallet/assets-list.tsx`:
- Around line 41-48: The Sort and Filter buttons in the assets-list component
are rendered without handlers; add minimal wiring by introducing local state
(e.g., sortKey, sortOrder, filterQuery) and functions named sortAssets and
filterAssets that update the displayed assets list, then attach these as
onClick/onChange handlers to the existing Button elements (the ones rendering
ArrowUpDown and Filter icons) so clicking Sort toggles sortOrder or opens a
simple sort menu and clicking Filter applies filterQuery; alternatively, if full
behavior isn't ready, mark the Button components as disabled with an explanatory
aria-label/title ("Coming soon") and add TODO comments referencing sortAssets
and filterAssets so future work can implement the stateful handlers.
- Around line 90-98: Precompute the portfolio total once (e.g., const totalUsd =
assets.reduce((acc, c) => acc + c.usdValue, 0)) and use that inside the JSX
instead of calling assets.reduce for every row; also guard both divisions:
compute unitValue as asset.amount ? asset.usdValue / asset.amount : 0 and
compute percent as totalUsd ? (asset.usdValue / totalUsd) * 100 : 0, then pass
unitValue to formatCurrency and percent.toFixed(1) for display, referencing
formatCurrency, asset.usdValue, asset.amount and assets.reduce to locate the
changes.
In `@components/wallet/security-section.tsx`:
- Around line 6-7: Remove the unused imports Label and Shield from the import
statement in components/wallet/security-section.tsx; update the import line that
currently imports Label and Shield (alongside other icons like Smartphone, Key,
etc.) so it only imports the symbols actually used in the file (e.g.,
Smartphone, Key, CircleAlert, CheckCircle2, ListFilter, MonitorPlay) to satisfy
the linter.
- Around line 37-41: The Switch is rendered as a controlled input with
checked={walletInfo.has2FA} but no onCheckedChange, so make its behavior
explicit: either mark the Switch disabled/readOnly when it’s meant for display
(e.g., add the disabled prop) or wire it to state by providing an
onCheckedChange handler that updates the relevant state/prop (update the parent
state or call a handler that toggles walletInfo.has2FA). Locate the Switch
component near Badge and adjust it to either include disabled (for display-only)
or add an onCheckedChange callback that updates walletInfo.has2FA.
In `@components/wallet/transaction-history.tsx`:
- Line 6: Remove the unused ExternalLink import from the import statement in
transaction-history.tsx; update the import line that currently reads importing {
Search, Download, ArrowUpRight, ArrowDownLeft, ExternalLink } (from
lucide-react) to omit ExternalLink so only used icons remain, which will resolve
the CI lint error.
- Around line 56-59: The Export CSV Button is rendered without a click handler;
either wire it to an export function or mark it disabled. Add a handler function
(e.g., exportTransactionsToCsv or handleExportCsv) in the TransactionHistory
component that gathers the transactions list (from props/state like
transactions), converts them to CSV rows/headers, creates a Blob/URL and
triggers a download, and attach it to the Button via onClick={handleExportCsv};
alternatively, if export is out of scope, set the Button disabled and add an
accessible title/aria-label to indicate unavailable functionality.
In `@components/wallet/wallet-overview.tsx`:
- Around line 5-55: The truncateStellarAddress import is unused and the full
address is rendered; update the WalletOverview component to display the
truncated address (use truncateStellarAddress(walletInfo.address)) where
{walletInfo.address} is currently rendered, keep the full address available via
a title/tooltip on that element (e.g., title={walletInfo.address}) so
copy/clipboard and accessibility still use the full value, and ensure the
truncateStellarAddress import is kept to satisfy the linter.
In `@components/wallet/withdrawal-section.tsx`:
- Line 8: Remove the unused Wallet icon from the import list to fix the lint
error: edit the import statement in withdrawal-section.tsx that currently
imports { Building, Plus, ArrowRight, Wallet, History, Info } and remove the
Wallet symbol so only used icons remain (Building, Plus, ArrowRight, History,
Info); ensure no other references to Wallet exist in the file.
- Line 102: The JSX text "You'll Receive" in the <span> within
withdrawal-section.tsx triggers react/no-unescaped-entities; fix it by escaping
the apostrophe or using a JS string expression — replace the raw text inside the
<span> (the element containing "You'll Receive") with either "You'll
Receive" or {"You'll Receive"} so the apostrophe is not an unescaped entity.
🧹 Nitpick comments (5)
components/wallet/wallet-sheet.tsx (1)
45-53: Consider extractingformatCurrencyto a shared utility.This helper duplicates logic that also exists in
helpers/format.helper.tsand is repeated inwithdrawal-section.tsx. Centralizing currency formatting ensures consistent formatting across the app and reduces maintenance burden.♻️ Suggested refactor
+import { formatCurrency } from "@/helpers/format.helper";Then remove the local
formatCurrencydefinition and update the existing helper if needed to match this implementation's behavior.components/wallet/withdrawal-section.tsx (4)
24-29: DuplicateformatCurrencyhelper.This is the same currency formatting logic duplicated in
wallet-sheet.tsx. Consider extracting to a shared utility inhelpers/format.helper.ts.
103-103: Edge case: negative receive amount displays as $0.00.The calculation
Math.max(0, (parseFloat(amount) || 0) - 2.50)correctly prevents negative values, but users entering amounts between $0.01-$2.49 will see "$0.00" receive amount while the button remains disabled only below $10. Consider adding a visual warning when the fee exceeds the amount.
107-110: Validation is correct but could include upper bound check.The button disables for amounts below $10, but there's no validation against the wallet balance or daily/monthly limits shown in the UI. Users could attempt to withdraw more than available.
♻️ Suggested enhancement
-<Button className="w-full h-12 text-md font-semibold" disabled={!amount || parseFloat(amount) < 10}> +<Button className="w-full h-12 text-md font-semibold" disabled={!amount || parseFloat(amount) < 10 || parseFloat(amount) > walletInfo.balance}>
19-22: Mock bank accounts are hardcoded.This is acceptable for initial implementation, but the comment on line 19 should be addressed before production. Consider adding a TODO with tracking context.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@components/wallet/transaction-history.tsx`:
- Around line 76-81: The search Input currently relies on placeholder text for
identification (component: Input with props placeholder, value={search},
onChange={(e) => setSearch(e.target.value)}); add an accessible label by either
providing an explicit aria-label prop (e.g., aria-label="Search transactions")
or by adding a visible or visually-hidden <label> tied to the Input via id and
aria-labelledby so screen readers can announce the field correctly.
- Around line 43-69: The CSV export (handleExportCsv) needs two fixes: sanitize
cells to prevent CSV injection by prefixing any cell that begins with "=", "+",
"-", "@" (or a tab) with a safe character (e.g., a leading single quote) when
building rows from filteredActivity, and ensure the created object URL is
revoked after download (use URL.revokeObjectURL(url) once the link click
completes or in a short setTimeout) so the Blob URL is not leaked; update the
rows/csvContent generation to apply the sanitization to each cell and call
URL.revokeObjectURL(url) after triggering the download and removing the link.
- Line 8: The code calls format(new Date(item.date), ...) directly (in
components/wallet/transaction-history.tsx at the two locations that construct
dates) which throws on invalid dates; add a small guard/helper (e.g., a local
formatSafeDate or use date-fns isValid) that builds a Date from item.date,
checks isValid(date) (or !isNaN(date.getTime())), and only calls format when
valid, otherwise return a safe fallback (empty string or a placeholder like
"—"); replace the two direct format(new Date(item.date), ...) usages with this
safe helper to prevent crashes when upstream date values are malformed.
In `@components/wallet/withdrawal-section.tsx`:
- Around line 16-18: The withdrawal form currently only checks amount >= 10,
allowing negative, non-numeric, or over-balance submissions; update the
validation in WithdrawalSection to (1) parse amount to a number (e.g., Number or
parseFloat), (2) ensure it is a finite positive number, (3) enforce min value
(>= 10) and max value <= walletInfo.balance, and (4) use this combined predicate
wherever the submit button is enabled/disabled and in the submit handler
(references: WithdrawalSection component, amount state, setAmount,
walletInfo.balance, and the submit/handleWithdraw functions) so both UI and
server-side call prevention are consistent.
🧹 Nitpick comments (5)
components/wallet/wallet-overview.tsx (3)
16-24: Consider cleanup on unmount to avoid React warnings.The
setTimeoutcallback may fire after the component unmounts, causing a no-op state update. This is harmless but can trigger React warnings in development.♻️ Optional: Clean up timeout on unmount
+import { useState, useRef, useEffect } from "react"; ... export function WalletOverview({ walletInfo }: WalletOverviewProps) { const [copied, setCopied] = useState(false); + const timeoutRef = useRef<NodeJS.Timeout | null>(null); + + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []); const handleCopyAddress = async () => { try { await navigator.clipboard.writeText(walletInfo.address); setCopied(true); - setTimeout(() => setCopied(false), 2000); + timeoutRef.current = setTimeout(() => setCopied(false), 2000); } catch (error) { console.error("Failed to copy address:", error); } };
32-38: Status is hardcoded rather than derived from wallet state.The status displays "Active & Secured" statically. Consider deriving this from
walletInfo.isConnectedandwalletInfo.has2FAfor accurate representation.
70-76: Creation date is hardcoded.The "Created" field shows a static "May 2024" value. Per PR objectives, this should display the actual wallet creation date. Consider adding a
createdAtfield toWalletInfotype.components/wallet/withdrawal-section.tsx (1)
19-23: Make destination selection a real, accessible control.
The account row is styled as interactive but has no selection state or keyboard semantics. Either remove the “clickable” affordance or wire up selection (recommended).♻️ Suggested refactor
export function WithdrawalSection({ walletInfo }: WithdrawalSectionProps) { - const [amount, setAmount] = useState(""); - // Mock bank accounts const bankAccounts = [ - { id: '1', name: 'Chase Bank', last4: '4242', isPrimary: true }, + { id: "1", name: "Chase Bank", last4: "4242", isPrimary: true }, ]; + const [amount, setAmount] = useState(""); + const [selectedBankId, setSelectedBankId] = useState(bankAccounts[0]?.id ?? ""); @@ {bankAccounts.map((account) => ( - <div key={account.id} className="flex items-center justify-between p-3 rounded-xl border border-primary bg-primary/5 cursor-pointer"> + <button + key={account.id} + type="button" + onClick={() => setSelectedBankId(account.id)} + aria-pressed={account.id === selectedBankId} + className={`flex w-full items-center justify-between p-3 rounded-xl border ${ + account.id === selectedBankId ? "border-primary bg-primary/5" : "border-border" + }`} + > <div className="flex items-center gap-3"> <div className="p-2 rounded-full bg-primary/10"> <Building className="h-4 w-4 text-primary" /> </div> @@ - {account.isPrimary && <Badge className="text-[10px] h-5">Primary</Badge>} - </div> + {account.isPrimary && <Badge className="text-[10px] h-5">Primary</Badge>} + </button> ))}Also applies to: 70-82
components/wallet/transaction-history.tsx (1)
24-32: Avoid recreatingIntl.NumberFormatper cell. Hoist the formatter to cut repeated allocations in large tables.♻️ Proposed refactor
+const usdFormatter = new Intl.NumberFormat("en-US", { + style: "currency", + currency: "USD", +}); + export function TransactionHistory({ activity }: TransactionHistoryProps) { @@ - const formatCurrency = (amount: number, currency: string) => { - if (currency === "USD" || currency === "USDC") { - return new Intl.NumberFormat("en-US", { - style: "currency", - currency: "USD", - }).format(amount); - } - return `${amount.toLocaleString()} ${currency}`; - }; + const formatCurrency = (amount: number, currency: string) => { + if (currency === "USD" || currency === "USDC") { + return usdFormatter.format(amount); + } + return `${amount.toLocaleString()} ${currency}`; + };
Contributor
|
@Michaelkingsdev resolve the coderabbit's comment |
Contributor
Author
|
@0xdevcollins |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #69
Wallet Page Implementation
This PR introduces a comprehensive, dedicated wallet page at
/walletto provide users with a complete view of their abstracted wallet. This extends the functionality of the existing wallet sheet, allowing contributors to manage their earnings, view detailed asset breakdowns, access full transaction history, and off-ramp funds to fiat currency.Core Features
Dedicated Wallet Hub (
/wallet)UI & UX Improvements
#a7f950) on white backgrounds using dynamic color switching (primary-foregroundin light mode).WalletSheetfor seamless transition to the full page.Technical Changes
New Route
/wallet/page.tsxNew Components
Mock Data
lib/mock-wallet.tsto includemockWalletWithAssetsfor realistic testing.How to Verify
/walletroute (either via direct URL or the new Wallet link in the navbar).Proof
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.